Skip to content

Changes for isuues 1413 and 1414 #1419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ilyamandel merged 5 commits into from
Aug 8, 2025
Merged

Changes for isuues 1413 and 1414 #1419

ilyamandel merged 5 commits into from
Aug 8, 2025

Conversation

@jeffriley
Copy link
Collaborator

@jeffriley jeffriley commented Aug 8, 2025 •
edited
Loading

Changes for issue 1413:

  • Deprecated options
    . "--minimum-mass-secondary" in favour of "--minimum-sampled-secondary-mass"
    . "--initial-mass-min" in favour of "--initial-mass-function-min"
    . "--initial-mass-max" in favour of "--initial-mass-function-max"
    . "--initial-mass-power" in favour of "--initial-mass-function-power"
  • Default value for "--initial-mass-function-min" remains at 5.0 Msol
  • Default value for "--initial-mass-function-max" remains at 150.0 Msol
  • Default value for "--initial-mass-function-power" remains at 0.0
  • Default value for "--minimum-sampled-secondary-mass" remains at 0.1 Msol
  • User supplied value for "--minimum-sampled-secondary-mass" checked against values of constants MINIMUM_INITIAL_MASS and MINIMUM_INITIAL_MASS
  • Secondary mass, whether input by user, sampled, or calculated from M1 & q, checked against constant MINIMUM-INITIAL-MASS
  • Online docs "program-options-list-defaults" and "standard-logfiles-record-specification-optios" pages updated
  • Whatsnew page updated

Changes for issue 1414:

  • Changed [SSE/BSE]_PULSAR_RECORDTYPE::DEFAULT to [SSE/BSE]_PULSAR_RECORDTYPE::PRE_SN
  • don't want DEFAULT anymore, but wanted to preserve numbering. Neither PRE_SN nor POST_SN are currently used (POST_SN was), so we could rename them one day.
  • Removed default parameter from BaseStar::PrintPulsarEvolutionParameters() and BaseBinaryStar::PrintPulsarEvolutionParameters()
  • Changed default record types for:
    - SSE and BSE detailed output files to include only record types 1, 4, & 5 (INITIAL_STATE, TIMESTEP_COMPLETED, and FINAL_STATE)
    - SSE and BSE pulsar evolution files to include only record type 3 ((pulsar) TIMESTEP_COMPLETED)

@ilyamandel I think I captured what you wanted for issue 1413 (I was interrupted and came back to the code changes a few times, but hopefully I remembered where I was when I was interrupted each time...). Let me know if I've forgotten anything or missed the mark.

@jeffriley jeffriley self-assigned this Aug 8, 2025
@jeffriley jeffriley added bug Something isn't working enhancement New feature or request severity_minor This bug is not very severe urgency_low This issue is not urgent labels Aug 8, 2025
This was linked to issues Aug 8, 2025
Minimum secondary mass #1413
Closed
Change to default record types in output files #1414
Closed
@jeffriley jeffriley requested a review from ilyamandel August 8, 2025 06:23
@jeffriley
Copy link
Collaborator Author

jeffriley commented Aug 8, 2025

I'm suspicious of what checks were actually run...

@ilyamandel
Copy link
Collaborator

ilyamandel commented Aug 8, 2025

Thanks, @jeffriley, will check tomorrow.

ilyamandel
ilyamandel approved these changes Aug 8, 2025
View reviewed changes
Copy link
Collaborator

@ilyamandel ilyamandel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, @jeffriley ! Just two minor things prior to merging this in:

The program option documentation for --initial-mass-function mentioned --initial-mass-function-min (and -max) only for UNIFORM; they are also used for SALPETER, KROUPA, and POWERLAW -- so would be more consistent to mention them for all or for none

At the bottom of the same file (online-docs/pages/User guide/Program options/program-options-list-defaults.rst), where we list all options, initial-mass-power should now be initial-mass-function-power

@jeffriley
Copy link
Collaborator Author

jeffriley commented Aug 8, 2025

Thanks @ilyamandel - I knew I'd miss something in the documentation :-)

I know my changes compile without errors (or warnings) here, but we don't seem to be doing the build check in github. I'm happy to merge (when I've fixed the documentation) - are you?

@ilyamandel
Copy link
Collaborator

ilyamandel commented Aug 8, 2025

I am happy to merge too; in fact, merging now. :-)

@ilyamandel ilyamandel merged commit 9e04db7 into dev Aug 8, 2025
2 checks passed
@ilyamandel ilyamandel deleted the issue-1413 branch August 8, 2025 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ilyamandel ilyamandel ilyamandel approved these changes

Assignees

@jeffriley jeffriley

Labels

bug Something isn't working enhancement New feature or request severity_minor This bug is not very severe urgency_low This issue is not urgent

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Change to default record types in output files Minimum secondary mass

3 participants

@jeffriley @ilyamandel